Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: OverKeyboardView with custom ShadowNode #863

Merged
merged 18 commits into from
Mar 25, 2025

Conversation

kirillzyusko
Copy link
Owner

@kirillzyusko kirillzyusko commented Mar 16, 2025

📜 Description

Stretch OverKeyboardView to full screen height on fabric/android.

💡 Motivation and Context

To make OverKeyboardView to stretch to full screen we have to override layout on ShadowNodes side. For paper architecture we already do that in corresponding ShadowNode kotlin class. Since new architecture is C++ based - we have to make corresponding changes in C++ code.

Unfortunately there is no easy way to add custom properties/code in existing/auto-generated shadow-nodes. To make it working we have to genereate them ourself and change the code accordingly. In this PR I did that. I copied autogenerated code, formatted it according to cpplint rules and did a proper linking. After that I wrote additional code that uses values passed from kotlin in c++ and properly resized the inner view.

Last, but not least - on iOS we manually specify the size of inner child, on Android - we are laid out by ShadowNodes. To keep the same behavior on Android we need to stretch to full screen width top: 0, right: 0, bottom: 0, left: 0, while on iOS we just have to specify exact dimensions.

Also on Android/Fabric I discovered, that if view keeps mounted, then the view intercepts touches (even if it's not laid out properly). To fix that and match RN behavior I added visible && children condition - in this case we don't mount children and thus they are not clickable 🙃

Closes #862

📢 Changelog

Example

  • added black color for "KYC" button (toolbar);

JS

  • removed architecture.ts file;
  • generate interfaceOnly from codegen;
  • don't mount children until visible=true (to match Modal behavior);
  • added react-native.config.js;

C++

  • created common folder that contains all ShadowNodes;

iOS

  • added common dependency in Podfile for new architecture;

Android

  • added jni folder;
  • added CMakeList.txt;
  • added custom detekt config;
  • pass StateWrapper to OverKeyboardView;
  • added stretchTo function to OverKeyboardView;

🤔 How Has This Been Tested?

Tested locally on Medium Phone API 35 (paper and fabric) and on CI via e2e tests.

📸 Screenshots (if appropriate):

Before After
image image

📝 Checklist

  • CI successfully passed
  • I added new mocks and corresponding unit-tests if library API was changed

Sorry, something went wrong.

@kirillzyusko kirillzyusko self-assigned this Mar 16, 2025
@kirillzyusko kirillzyusko added 🏭 fabric Changes specific to new (fabric/jsi) architecture OverKeyboardView Anything related to OverKeyboardView 🐛 bug Something isn't working labels Mar 16, 2025
Copy link
Contributor

github-actions bot commented Mar 16, 2025

📊 Package size report

Current size Target Size Difference
173342 bytes 170428 bytes 2914 bytes 📈

@kirillzyusko kirillzyusko changed the title feat: OverKeyboardView custom ShadowNode feat: OverKeyboardView with custom ShadowNode Mar 16, 2025
@kirillzyusko kirillzyusko force-pushed the feat/over-keyboard-view-custom-shadow-node branch from 3434212 to 7082b59 Compare March 19, 2025 10:35
@kirillzyusko kirillzyusko marked this pull request as ready for review March 25, 2025 10:17
@kirillzyusko kirillzyusko merged commit 03937a7 into main Mar 25, 2025
26 checks passed
@kirillzyusko kirillzyusko deleted the feat/over-keyboard-view-custom-shadow-node branch March 25, 2025 10:18
@Mako-L
Copy link
Contributor

Mako-L commented Mar 25, 2025

This looks amazing! @kirillzyusko When we will have a new version of the package including this?

@kirillzyusko
Copy link
Owner Author

@Mako-L I will publish it under 1.17.0, and it may take several weeks (I want to merge some other PRs and then release 1.17.0).

In the meantime if you want you can use dependency from a main branch (directly from github).

@Mako-L
Copy link
Contributor

Mako-L commented Mar 26, 2025

@kirillzyusko I just tried the package from main branch and now there is other issue the 'TouchableWithoutFeedback' from example in documentation press doesn't trigger on all the screen it only runs the tap function only on the bottom like around 200px height or something like that. I will open a new ticket and update my repo to reproduce the bug. Thanks again.

Edit:
On demo code apparently it works when i put it in new repo so I must be doing something wrong in my main code. Sorry for the misunderstanding.

@kirillzyusko
Copy link
Owner Author

@Mako-L yeah, I tested example app a lot to be sure that everything works identical to how it worked on paper

But let me know if you discover any issues! I'll be happy to fix them before a new release ☺️

@Mako-L
Copy link
Contributor

Mako-L commented Mar 26, 2025

@kirillzyusko The problem appears when using KeyboardStickyView with OverKeyboardView. Opened another report: Issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🏭 fabric Changes specific to new (fabric/jsi) architecture OverKeyboardView Anything related to OverKeyboardView
Projects
None yet
2 participants